Skip to content

Issue 435: run_solver.py re-structuring and hipo inclusion#454

Closed
finozzifa wants to merge 65 commits into
mainfrom
issue_435_run_solver_hipo
Closed

Issue 435: run_solver.py re-structuring and hipo inclusion#454
finozzifa wants to merge 65 commits into
mainfrom
issue_435_run_solver_hipo

Conversation

@finozzifa
Copy link
Copy Markdown
Member

@finozzifa finozzifa commented Feb 25, 2026

Issue solved

The pull request solves issue #435 (which tackles parts of issue #386).

Changes

The changes that are performed in this pull request are (on a high level).

Changes to run_solver.py

The changes are:

  • rs_change 1: I have introduced a new Enum class called HighsSolverVariant which contains the possible solver choices that highspy supports
  • rs_change 2: I have introduced a new function called def parse_args() that implements a command-line argument parser.
  • rs_change 3: I removed the code introduced in pull request Runner: add support to build & run HiGHS HiPO solver #388, as hipo can be called by passing the necessary option to argument solver. In particular, the HiPO variants previously contained in the Enum class called HighsVariant have been removed. This is because it is now possible to pass the HiPO block size as the CLI argument --hipo_block_size
  • rs_change 4: I have added numpydoc-style docstring and static type annotations.
  • rs_change 5: On a high level, the flow within the script is now:
    • Parse CLI arguments with def parse_args()
    • Instantiate and configure a solver object based on the specified solver name and options with def get_solver. The function sets the solver options and seed options respectively with def set_solver_options and def set_seed_options
    • Solves the benchmark instance and returns the result metrics.

Furthermore I have updated the README.md with the new proposed changes to run_solver.py.

Changes to run_benchmarks.py

The changes are:

  • rb_change 1: I have added numpydoc-style docstring and static type annotations.
  • rb_change 2: I have modified the function def download_benchmark_file and placed the logic for downloading a file over HTTP(S) requests to the function def _download_via_requests, the logic for downloading a file from google cloud storage (via gsutil) to def _download_via_gsutil and the logic to decompress a gzip file and remove the original compressed file to def _unzip_gz. The three functions are invoked in download_benchmark_file.
  • rb_change 3: My understanding is that the reference benchmark was previously executed by means of the HiGHS binary. Given the fact that the binary is now included in highspy==1.13.2.dev1, I have removed the function benchmark_highs_binary. The reference benchmark is now run by means of def benchmark_solver using highs-hipo
  • rb_change 4: Given the fact that the HighsVariant Enum has been removed and its content parsed differently in run_solver.py (please see the rs_change 3), I have removed get_highs_binary_version
  • rb_change 5: I have extended def benchmark_solver to run also the reference benchmark with highs-hipo. Moreover, I have placed the logic to interpret a subprocess CompletedProcess from a solver run and produce a metrics dictionary to the function parse_solver_result, the logic to build a metrics dictionary for solver failure cases to the function def return_failure_metrics and the logic to build the shell command to run a solver with resource limits to the function def build_solver_command. The three functions are invoked in benchmark_solver. In particular, def build_solver_command accounts for the changes in the way run_solver.py is invoked.

Addition of benchmark-2026.yaml

I have created a new environment file runner/envs/benchmark-2026.yaml which includes the channel conda-forge/label/dev where highspy==1.13.2.dev1 is available. The environment file will need to be modified once a new version of highspy containing HiPO is published to conda-forge.

Tests

Unit tests

When studying and understanding a new code-base, I find it helpful to write unit tests as I go. Tests serve as a kind of executable documentation. I followed this approach while working on run_benchmarks.py. I placed the unit tests under tests/test_run_benchmarks.py. Rather than removing them afterwards, I have decided to keep them in the repository for everyone's benefit. If not needed, they can be of course removed 😉

Tests for run_solver.py

I perfomed the tests with knitro and xpress on a "quick" benchmark, because I just wanted to verify that the new version of run_solver.py was still working with these two commercial solvers.

As for highs I instead performed the tests on the benchmark that was proposed in issue #386 .

Test with Knitro

Benchmark instance: pypsa-eur-elec-trex-3-12h.lp
Machine for the test: local test on a laptop
Results: the objective function is the same one found in pull request #410 (run time is different as the tests performed in #410 were done on the benchmark_gen GCP virtual machine)

python runner/run_solver.py --solver_name knitro --solver_version 15.1.0 --input_file ~/files/pypsa-eur-elec-trex-3-12h.lp
...
{"runtime": 5.228895955000098, "reported_runtime": 4.0366621017456055, "status": "ok", "condition": "optimal", "objective": 7398201117.411588, "duality_gap": null, "max_integrality_violation": null}

Test with XPRESS

Benchmark instance: pypsa-eur-elec-trex-3-12h.lp
Machine for the test: local test on a laptop
Results: the objective function is the same one found in pull request #362 (run time is different as the tests performed in #362 were done on the benchmark_gen GCP virtual machine)

python runner/run_solver.py --solver_name xpress --solver_version 9.8.0 --input_file ~/files/pypsa-eur-elec-trex-3-12h.lp
...
{"runtime": 1.7614182239999536, "reported_runtime": 1.35, "status": "ok", "condition": "optimal", "objective": 7398190831.303641, "duality_gap": null, "max_integrality_violation": null}

Test with Highs IPX

Benchmark instance: pypsa-de-elec_6_1h.mps.gz
Machine for the test: test on benchmark_gen GCP VM
Results: the command line output is available at ipx_run.txt

python runner/run_solver.py --solver_name highs --solver_version 1.13.2.dev1 --input_file ~/files/pypsa-de-elec_6_1h.mps --highs_solver_variant ipx

{"runtime": 6336.666104405, "reported_runtime": 6321.194668531418, "status": "ok", "condition": "optimal", "objective": 5581029701.026945, "duality_gap": null, "max_integrality_violation": null}

Test with Highs HiPO

Benchmark instance: pypsa-de-elec_6_1h.mps.gz
Machine for the test: test on benchmark_gen GCP VM
Results: the command line output is available at hipo_run.txt

python runner/run_solver.py --solver_name highs --solver_version 1.13.2.dev1 --input_file ~/files/pypsa-de-elec_6_1h.mps --highs_solver_variant hipo --hipo_block_size 64

{"runtime": 1067.0535390559999, "reported_runtime": 1051.7138698101044, "status": "ok", "condition": "optimal", "objective": 5581029701.107754, "duality_gap": null, "max_integrality_violation": null}

Test with CBC

Benchmark instance: tests/sample_benchmarks/sample_lp.lp and tests/sample_benchmarks/sample_mip.lp
Machine for the test: local test on a laptop
Results:

python runner/run_solver.py --solver_name cbc --solver_version 2.10.12 --input_file  tests/sample_benchmarks/sample_lp.lp

{"runtime": 0.04282720799892559, "reported_runtime": null, "status": "ok", "condition": "optimal", "objective": 11.75, "duality_gap": null, "max_integrality_violation": null}
python runner/run_solver.py --solver_name cbc --solver_version 2.10.12 --input_file  tests/sample_benchmarks/sample_mip.lp

{"runtime": 0.38361791699935566, "reported_runtime": 0.0, "status": "ok", "condition": "optimal", "objective": 30.0, "duality_gap": null, "max_integrality_violation": 0}

Tests for run_benchmarks.py

I cannot test run_benchmarks.py on my laptop because the script relies upon systemd-run which is not available on osx-arm64. I therefore tested it through the CI.

The runs on tests/sample_benchmarks/sample_lp.lp and tests/sample_benchmarks/sample_mip.lp failed for the cbc solver, even though I can successfully run these instances locally with cbc.

python runner/run_solver.py --solver_name cbc --solver_version 2.10.12 --input_file  tests/sample_benchmarks/sample_lp.lp

{"runtime": 0.04282720799892559, "reported_runtime": null, "status": "ok", "condition": "optimal", "objective": 11.75, "duality_gap": null, "max_integrality_violation": null}
python runner/run_solver.py --solver_name cbc --solver_version 2.10.12 --input_file  tests/sample_benchmarks/sample_mip.lp

{"runtime": 0.38361791699935566, "reported_runtime": 0.0, "status": "ok", "condition": "optimal", "objective": 30.0, "duality_gap": null, "max_integrality_violation": 0}

For the moment I have therefore removed cbc from the default solvers that are invoked by benchmark_all.sh.

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
solver-benchmark Ready Ready Preview, Comment Mar 5, 2026 3:36pm

Request Review

@finozzifa finozzifa requested a review from eantonini February 25, 2026 14:33
@finozzifa finozzifa changed the title Issue 435: run_solver.py re-structuring and hipo inclusion DO NOT MERGE - Issue 435: run_solver.py re-structuring and hipo inclusion Feb 25, 2026
@siddharth-krishna siddharth-krishna linked an issue Feb 26, 2026 that may be closed by this pull request
3 tasks
Comment thread runner/run_solver.py Outdated
Comment thread runner/run_solver.py
@finozzifa finozzifa changed the title DO NOT MERGE - Issue 435: run_solver.py re-structuring and hipo inclusion Issue 435: run_solver.py re-structuring and hipo inclusion Mar 5, 2026
Copy link
Copy Markdown
Member

@siddharth-krishna siddharth-krishna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Fabrizio! I notice one potential issue: reference benchmarks should be run with the reference Highs binary, not HiPO. Let me know if you'd like to discuss this or have any questions.

I would also like to gently encourage future PRs to be smaller / focused on a single logical change when possible. In particular, mixing refactoring with logic changes makes the PR larger and harder to review. If you agree, let's do it that way in the future please.

Comment thread runner/benchmark_all.sh
Comment thread runner/run_benchmarks.py
Comment thread runner/run_benchmarks.py
Comment on lines +460 to +462
reference_benchmark : bool
If True, appends the ``--highs_solver_variant hipo`` flag to run
the HiGHS HiPO variant on a reference instance.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain this? So far when we run the reference benchmark we don't run HiPO, we run a static binary of HiGHS with default options.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Sid. I thought the static binary was the one for HiPO, given the fact that HiPO was not available in highspy. Should we keep using the static binary?

Comment thread runner/run_benchmarks.py
Comment on lines +539 to +548
reported_runtime = runtime if isinstance(runtime, (int, float)) else None
return {
"status": status,
"condition": condition,
"objective": None,
"runtime": runtime,
"reported_runtime": reported_runtime,
"duality_gap": None,
"max_integrality_violation": None,
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love well documented code as much as anyone, and this PR hugely improves the documentation of this script, but is it always worth breaking things into functions? In this case, I'm not sure it makes things more readable, and we've increased the size of this code by 2-3x with the new function and docstring. What do you think?

Comment thread runner/run_benchmarks.py


def parse_memory(output):
line = output.splitlines()[-1]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line got removed -- are we sure the new code has the same behavior?

Comment thread runner/run_benchmarks.py
Comment on lines +680 to +683
if not log_file.exists():
print(f"Creating missing log file {log_file}")
log_file.touch()
with open(log_file, "a") as f:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log file is created by run_solver btw. This change is fine too.

Comment thread runner/run_benchmarks.py
Run a reference benchmark using the pre-installed HiGHS binary
"""
reference_model = "/benchmark-test-model.lp"
highs_binary = "/opt/highs/bin/highs"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function shouldn't be removed. We use this to estimate the runtime variance of each VM.

Comment thread runner/run_solver.py

solver_class = getattr(solvers, solver_enum.name)

def set_seed_options(solver_name: str) -> dict[str, int | float]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_solver_options?

Comment thread runner/run_solver.py
return None


def run_highs_hipo_solver(input_file, solver_version, highs_variant: HighsVariant):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this function for estimating runtime variation using the reference binary solver and benchmark

@finozzifa
Copy link
Copy Markdown
Member Author

Ok it seems I misunderstood what we were supposed to do. I think it is worth re-starting from scratch. Closing this PR.

@finozzifa finozzifa closed this Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runner: re-work run_solvers.py for using HiPO conda package

3 participants